Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add Preview/Draft Layout Wrapper for Testing Links #4090

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: Add Preview/Draft Layout Wrapper for Testing Links #4090

wants to merge 11 commits into from

Conversation

RODO94
Copy link
Contributor

@RODO94 RODO94 commented Dec 18, 2024 •
edited
Loading

What does this PR do?

This PR adds a wrapper around the Preview and Draft view to initially show a 'This is a test environment' node that a user would have to navigate past.

For this PR, I used the Layout pattern as a guide, but just used a Page rather than a Layout to wrap the View. I started with the Layout in place of the Page below but it didn't contain any extra logic or anything useful, so I removed it.

  return (
    <PublicLayout>
      <TestWarningPage>
        <View />
      </TestWarningPage>
    </PublicLayout>
  );

I used the Save and Return journey as a guide and created a new component inside the Card component to control which Or route is used. This has all been decoupled from the Card component and added to the OrNavigationButton

       <OrNavigationButton handleSubmit={handleSubmit} />

Hopefully this is an elegant solution and I have overcomplicated this <OrNavigationButton>. An unintended consequence of this is that the Go to live link is now present as an Or option on the Send component. This is why I had to mock the react-navi import for the tests

Important

When clicking "Go to published..." the flow data is deleted from localStorage and window is reset similar to the Restart button while you are navigated to the published?analytics=false page

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 18, 2024 •
edited
Loading

Pizza

Deployed 12187b9 to https://4090.planx.pizza.

Useful links:

@RODO94 RODO94 requested a review from a team December 19, 2024 09:21
@RODO94 RODO94 marked this pull request as ready for review December 19, 2024 09:21
DafyddLlyr
DafyddLlyr requested changes Dec 19, 2024
View reviewed changes
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

A few comments to take a look at 👍

editor.planx.uk/src/@planx/components/shared/Preview/NavigateToPublishedButton.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/@planx/components/shared/Preview/NavigateToPublishedButton.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/pages/Preview/TestWarningPage.tsx Outdated Show resolved Hide resolved
@RODO94 RODO94 force-pushed the rory/preview-wrapper branch from 30fbf51 to a2d1a18 Compare January 6, 2025 11:10
@RODO94 RODO94 requested a review from a team January 6, 2025 16:46
@jessicamcinchak
Copy link
Member

One question related to this point:

Important
"Go to the live service" remains across each node and when you click it, it will navigate you to that node in the flow rather than back at the start with pre-filled in questions

What happens when the /preview or /draft links have a node that is not in the published version? (As is most commonly the case? think this could lead to very strange and opaque submisison data errors?)

I don't think I actually agree that this is intended behavior, and I think users should be dropped at the beginning of a published service when clicking this link rather than midway?

@RODO94
Copy link
Contributor Author

RODO94 commented Jan 7, 2025

One question related to this point:

Important
"Go to the live service" remains across each node and when you click it, it will navigate you to that node in the flow rather than back at the start with pre-filled in questions

What happens when the /preview or /draft links have a node that is not in the published version? (As is most commonly the case? think this could lead to very strange and opaque submisison data errors?)

I don't think I actually agree that this is intended behavior, and I think users should be dropped at the beginning of a published service when clicking this link rather than midway?

@jessicamcinchak yeah, I wasn't sure on this, thanks for the clarification, I'll reset back to start on this

@RODO94 RODO94 requested a review from jessicamcinchak January 7, 2025 11:23
@RODO94 RODO94 force-pushed the rory/preview-wrapper branch from 0203d71 to c69cf29 Compare January 7, 2025 11:25
@RODO94 RODO94 requested a review from DafyddLlyr January 7, 2025 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jessicamcinchak jessicamcinchak Awaiting requested review from jessicamcinchak

@DafyddLlyr DafyddLlyr Awaiting requested review from DafyddLlyr

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RODO94 @jessicamcinchak @DafyddLlyr